The vault paid nothing, and then told him the pool was empty - #235
Merged
Conversation
OWNER, week 202632: he found the Paper, found the Golden Key, entered the right
code, and no USDT arrived. He recovered it only by reading the contract on
celoscan himself. A player cannot do that — for them the sequence ends at "won,
paid nothing".
WHAT WAS MEASURED BEFORE ANY CODE WAS WRITTEN, none of it guessed:
pool balance ............... 0.85 USDT (the reward is 0.05) enough
deposited 1.00 / claimed 0.15 ................................ enough
backend signer ............. 0.645 CELO enough
isCodeSetForWeek(202632) ... true
simulate submitVaultCode ... does NOT revert
celoscan ................... "Store Weekly Vault Code" SUCCEEDED,
"Submit Vault Code" NEVER BROADCAST
Not a revert, not funding, not permissions. The payout transaction never
existed. finalizeVaultPayout() did TWO sequential on-chain writes in ONE player
request — store this week's code, then pay — and only for the FIRST winner of
any given week. Everyone after them found the code already stored and took a
single write, which is why it looked random and survived weeks. Forno is
load-balanced, so the node answering the nonce read for write #2 need not have
seen write #1; the transaction was rejected at the RPC boundary before it was a
transaction. The catch swallowed it and reported "pending".
THE FIX IS TO DELETE THE SHAPE, NOT TO HARDEN IT.
Storing the code is now its own errand, done when the vault door is OPENED by
anybody — hours or days before a code is solved. POST /api/vault/prepare, fired
and forgotten by openVaultWindow(), plus the daily cron as a net for a week
nobody opens a door in. By the time someone wins, the payout is one write. The
two-write path is not more reliable; for real players it is gone.
When it is unavoidable — the first person to open this week's vault also solves
it in the same request — storeThenPay() reads the nonce ONCE and numbers both
writes locally. The RPC is never asked twice, so it cannot answer twice
differently. The payout also retries three times with backoff, re-checking
hasClaimedThisWeek before each one so a receipt we failed to hear is never paid
twice.
A REWARD CAN NO LONGER BE LOST BY NOT REACHING THE DOOR AGAIN.
The self-heal on re-open was the ONLY retry, and it is gated behind re-entering
Bunker 5, clearing it and killing the final boss. sweepPendingVaultPayouts()
now collects unpaid wins from the cron that already runs daily, across this week
AND last week so a Sunday-night win is not stranded by the Monday rollover. The
player does nothing.
AND THE MESSAGE STOPS ACCUSING THE POOL.
Every failed payout printed "the transfer completes as soon as the reward pool
is topped up". The server had never read the pool. That sentence is what sent
the owner, and then an agent, into the treasury for an hour while a dropped
transaction sat unexamined. Each cause is now CHECKED before it is named —
pool_empty reads getAvailableVaultFunds against vaultReward, signer_out_of_gas
reads the signer's balance — and the true one is sent down and printed.
Verified: 51/51 test:vaultpay (new; its FAILS-ON-OLD sections run the deleted
implementation against the same fakes and it fails all four), 18/18
test:vaultwin, 8/8 test:kills, 19/19 test:seasonxp, 128/128 test-season-close,
tsc clean, build succeeds with /game unchanged at 143/242 kB, lint 35 (baseline),
check:copy and audit clean, engine minifies 14/14.
NOT verified, and the test file says so in its header: there is no Celo node in
this sandbox, so every client in the new tests is a fake whose behaviour I chose.
They prove the code does the right thing when an RPC misbehaves the way the
chain evidence says it did. They do not prove that is the only way it can. The
sweep exists because I cannot promise this was the last failure mode.
Contracts untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLrEEtpTAcJq334heqKGit
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
0xward
added a commit
that referenced
this pull request
Aug 3, 2026
Two reasons, and the second is the one that needed doing. The branch was cut before #235 landed, so it was describing a repo that has since gained lib/server/vaultChain.ts and a vault payout path that no longer does two on-chain writes in one request. A documentation branch that is behind main is documentation that is already wrong. It also retriggers the preview deployment. The one for d8297cf errored with a build log that stops dead after 'Cloning completed: 50.039s' — no install, no next build, no error, nothing. Zero build output on a diff that adds one markdown file, while the GitHub checks workflow passed on the same SHA and every other deployment that day went green. That is a stall on Vercel's side, not something in the change, and it never retried on its own.
0xward
added a commit
that referenced
this pull request
Aug 3, 2026
The branch was cut before #235, so the file described a repo that no longer exists: 44 routes (now 45), a lib/server/ list with no vaultChain or vaultSweep, a cron that only closes seasons, and no mention of test:vaultpay. Three conventions added to §6, because they are exactly the kind this file is for — each one is a bug that already shipped and would have been avoided by a sentence: - two on-chain writes in one player request is how a solved vault paid nothing; the second write was rejected at the RPC boundary before it was a transaction, and only the first winner of each week ever took that path - anything that can leave a reward unpaid needs a server-side sweep, because the vault's only recovery was re-clearing Bunker 5 to reach the door - never name a cause you did not check: the payout blamed an empty pool it had never read A documentation branch that is behind main is documentation that is already wrong, which is the failure invariant #2 in this same file exists to name.
0xward
added a commit
that referenced
this pull request
Aug 3, 2026
…lib/ (#234) Every convention in this repo that matters is written down somewhere, and no two of them are in the same place. The engine boundary is explained in the header of lib/constants/game-config.ts. The invariants are §10 of a 67KB design doc. The reason CI does not run lint is a comment in a workflow file. The reason wagmi mounts as a sibling rather than a wrapper is thirty lines of Web3Providers.tsx, and that note exists because somebody had already undone it once. That is fine for whoever wrote all of it. It is not fine for an assistant opening the repo cold, which is exactly who keeps being handed these tasks: it reads the file it was pointed at, does not read the four other files explaining why that file is shaped the way it is, and helpfully simplifies something back into the bug it was written to fix. CLAUDE.md collects the parts that are load-bearing and cheap to violate: THE BOUNDARY, first, because everything else follows from it. Fourteen plain <script> files under public/ that next build never touches, cannot import from lib/, and whose syntax errors ship. The duplicated marketplace list is a consequence, not an oversight, and the engine's copy wins. THE INVARIANTS from GAME-DESIGN.md §10 verbatim, plus the ones the tooling already enforces but nobody would guess: one owner for the RPC endpoint and one for the origin, and an attribution tag that CANNOT be backfilled. WHAT THE CHECK SCRIPTS ARE. Not a test suite. Each one is a bug that already shipped, wearing a filename. check:cssvars exists because a primary button rendered as near-black text on a dark modal; check:copy exists because "Add Cash" reached four files and a "Connect wallet" line sat permanently on /profile. Listing them next to what they guard is what stops the next person from deleting the noisy one. THE MINIPAY RULES that live in copy and in .npmrc, which look like style preferences and are listing-review rejections. THE COMMIT STYLE, described from the log rather than prescribed. THREE SERVER RULES from the vault payout work in #235, added before this merged because a documentation branch that is behind main is documentation that is already wrong: never put two on-chain writes in one player request; anything that can leave a reward unpaid needs a server-side sweep; never name a cause you did not check. Verified against the source rather than the README: the env section names FIREBASE_SERVICE_ACCOUNT, which is what firebase-config.ts actually reads, while the README says FIREBASE_SERVICE_ACCOUNT_JSON, which nothing does. Route count, engine script count and load order, gitignored min/ output, and the audit's one-place-allowed rules for celoRpc.ts and siteUrl.ts were each read off the files. No source file changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes items 1–4 of the Treasure Vault report. No contract is touched.
What was measured before any code was written
Week 202632, the owner's own wallet — none of this is guessed:
isCodeSetForWeek(202632)truesubmitVaultCodeNot a revert, not funding, not permissions. The payout transaction never existed.
finalizeVaultPayout()did two sequential on-chain writes in one player request — store this week's code, then pay — and only for the first winner of any given week. Everyone after them found the code already stored and took a single write, which is why it looked random and survived weeks. Forno is load-balanced, so the node answering the nonce read for write #2 need not have seen write #1; the transaction was rejected at the RPC boundary before it was a transaction. Thecatchswallowed it and reported "pending".1. Storing the code is no longer part of paying
The priority item, and the reason the other three are small.
POST /api/vault/prepareputs the week's code on chain when the vault door is opened by anybody — hours or days before a code is solved — fired and forgotten byopenVaultWindow(), with the daily cron as a net for a week nobody opens a door in. By the time someone wins, the payout is one write.The two-write path is not made more reliable. For real players it is gone.
3. One nonce read when two writes are unavoidable
The degenerate case that remains: the first person to open this week's vault also solves it in the same request.
storeThenPay()reads the nonce once and numbers both writes locally, so the RPC cannot answer twice differently. The payout also retries three times with backoff, re-checkinghasClaimedThisWeekbefore each attempt — a receipt we failed to hear is settled asalready_claimed, never paid twice.2. A reward can no longer be lost by not reaching the door again
The self-heal on re-open was the only retry, and it is gated behind re-entering Bunker 5, clearing it and killing the final boss. The owner got his reward back by reading the contract on celoscan; a player cannot.
sweepPendingVaultPayouts()now collects unpaid wins from the cron that already runs daily, across this week and last week so a Sunday-night win is not stranded by the Monday rollover. The player does nothing.4. The message stops accusing the pool
Every failed payout printed "the transfer completes as soon as the reward pool is topped up". The server had never read the pool. That sentence is what sent you, and then an agent, into the treasury for an hour while a dropped transaction sat unexamined.
Each cause is now checked before it is named —
pool_emptyreadsgetAvailableVaultFundsagainstvaultReward,signer_out_of_gasreads the signer's balance — and the true reason is sent down and printed. Seven reasons, seven sentences, and only one of them is allowed to mention the pool.Verified
51/51 test:vaultpay(new) ·18/18 test:vaultwin·8/8 test:kills·19/19 test:seasonxp·128/128 test-season-close·tscclean · build succeeds,/gameunchanged at 143/242 kB · lint 35 (baseline, unchanged) ·check:copyandauditclean · engine minifies 14/14.The new test's FAILS-ON-OLD sections run the deleted implementation, reconstructed verbatim, against the same fakes — it fails all four, so "this test would have caught it" is demonstrated rather than claimed.
What these tests do NOT prove
Stated in the test file's own header, not only here. There is no Celo node in this sandbox and no backend key, so every client in the new tests is a fake whose behaviour I chose. They prove the code does the right thing when an RPC misbehaves in the way the chain evidence says it did. They do not prove that is the only way it can misbehave. The real verification is the next vault win on mainnet — and the daily sweep exists precisely because I cannot promise this was the last failure mode.
Question before merge
I could not reproduce Paper and Golden Key being consumed on a win. Server-side,
paperClaims/andgoldenKeyClaims/are written once per week and removed by nothing except guest migration;refreshVaultRequirements()reads those same records, so a returning player should still see ✓ ✓ all week. The barrier I could find is the one the sweep now removes: the vault door itself is only reachable by re-clearing Bunker 5.If what you saw was the Paper disappearing from the inventory panel, that is a different mechanism and I would rather you point me at it than have me guess.
Generated by Claude Code